-
Notifications
You must be signed in to change notification settings - Fork 701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Variable-based CPE and UFW rules for Ubuntu 20.04 STIG #7635
Conversation
Hello @dodys! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2022-02-10 16:05:57 UTC |
Hi @dodys. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
rebased to fix conflict in shared/macros-highlevel.jinja |
Well, happy to see this upstreamed, thanks @dodys! :-) Not really sure where to start any more. :D Variable based pseudo-CPEs was a solution we came upon early in the development of Canonical's internal CaC port. One feature we wanted was the ability to use a XCCDF variable to switch between one of various choices, but for sure audit against it. That is, compared to the real CPE system, even if the package is not installed (or could reasonably co-exist), we still want to generate pass/fail messages for the user's selection -- and preferably generate not-applicable (really: pass) messages for everything else. Note that not-applicable isn't possible due to a certain long-standing OVAL bug that still hasn't gotten solved. This further has the consequence that, if a XCCDF variable pseudo-CPE selection does NOT align with the packages on the system, the remediation of the benchmark would result in following user's desires, though it might take two scan+remediate passes in order to do so (given that CPE and pseudo-CPE could co-exist in the rule). That is, if UFW is selected in the XCCDF variable but iptables was present, and given that all rules included both CPEs and pseudo-CPEs, the iptables would pass (see note below -- less than ideal but given our constraints) and all other would return not-applicable (except the Since the CPE evaluation is independent of XCCDF variables (which is to say, CPE evaluation has no access to XCCDF variables), we end up creating a "pseudo-CPE": shared OVAL definitions placed in the main OVAL content. Each rule with a pseudo-CPE selector then has its OVAL modified to essentially include a block like so: <criteria operator="OR">
<criterion id="not_variable">
<criteria operator="AND">
<criterion id="variable">
....existing criteria/criterion....
</criteria>
</criteria> Ideally the first criterion would result in a hard not-applicable, but due to OVAL-Community/OVAL#24, this isn't possible. Thus, the first criterion results in a "pass". This gets put into both bespoke and templated OVALs. I think we ended up using pseudo-CPEs in a couple of places internally:
Notably, in both of these categories, two or more of the selections could co-exist peacefully on the system (firewalld and ufw are both front-ends to either iptables/nftables and thus definitely could co-exist with them --- or systemd-tymesyncd could be installed and inactive and co-exist with either of chrony/ntpd). Without XCCDF variable selector, alignment with the benchmarks (which, as of time of creation, audited firewall frontends rather than backends) is harder, if we want to preserve that users still need to make an explicit choice. Finally, note that this support only applies to the OVAL. Since SCE could in theory be any number of languages, and it is easy enough to provision+read a variable and return not-applicable manually, SCE does not have automatic pseudo-variable CPE support. IMO, if we restrict SCE to purely bash, this could be useful, but if we wish to support any other language (e.g., Python), it grows to become too complex to do automatically. Hope that helps to provide some context on the design and requirements for this system. |
Thanks for a detailed explanation, I think the feature can be a great improvement. Currently, @vojtapolasek and @yuumasato are working on some changes in CPEs, so it might be a good idea to consult this with them. @vojtapolasek or @yuumasato What are your thoughts on this proposal? |
With
I guess this issue is mostly caused because rules that configure a certain package have the CPE for applicability of said certain packages.
I did a local build and none of the ufw checks had the above construct, but the remediations had the check for variable=package.
Not sure I understand, could you clarify why is it hard to align the benchmark? I guess the XCCDF way of making a choice is via profile tailorings, and actually making a choice via XCCDF variable will in fact require a tailoring. It looks like the variable-based CPE is trying to generalize the approach that some SSHD rules have with regards to "system install status" and "profile intention" (or user intention). These "sshd requires" mechanism were implemented by me a long time ago based on some quirky requirements (that may actually no have been true, meaning they can probably be removed.), they are a bit convoluted and confusing I admit. But it seems to me that the proposed variable-based CPEs can get quite complex and confusing as well. I kinda got confused by "CPE" in the naming. I understand the proposed mechanism as a way to bundle up a few rules in a choice mechanism, but really determining applicability (validity or relevancy) of the rules. I see the difficulty in making available "choices" aligned with the policy, and see value in trying to make it easier for users to pick those choices. But I have a slight feeling this is adding a bit more complexity than necessary. |
I don't think this proposal affects the CPE Applicability being worked on by Vojta. |
Many thanks for the thoughtful comments Watson! @yuumasato said:
Close. Consider these rules (relevant for a benchmark):
In a CPE-only scenario, we'd have to make a choice here as to which rules to select in the profile. Let's assume the profile favors
Now consider the case where we use variable-based pseudo-CPEs. Here, we add all rules relevant to the benchmark to the profile. That means all of the rules listed above. Let's again assume
Notably, what this means is that a the tailoring merely contains organization-specific policy -- and NOT benchmark+organization-specific policy (in the case of CPE-only -- all This doesn't lose any of the ability for CaC to be opinionated about auditing/remediation of the benchmark (it can still favor whatever approach/tools it desires), and CPE is still present and IMO, useful.
Right, and with pseudo-variable based CPEs, we can now automate them and they only get executed when this variable says so. :-)
Right, in the remediation it should all be
Exactly. When you build CaC, you'd get dozens of rules for a single firewall. CIS, STIG, ... everything all creates slightly different firewall rules and gets dumped into the datastream. When the sysadmin wants to change from the default firewalling agent to another ( Also, this is still a profile tailoring: it just changes it from a choice of N benchmark required rules + K organization policy rules, to a single variable change + K organization policy rules -- meaning the intersection/interaction of the N+K rules isn't muddied.
Yeah, the name "pseudo-CPE" came about because originally, we were going to have the rules not selected (by the variable_ return "not applicable" -- just like CPE would've if the package wasn't installed. It was more of mimicing the functionality of CPE (from a user perspective) than the implementation from a benchmark creator's perspective. However, OVAL doesn't allow that right now. :/ Other name suggestions welcome :-)
Yeah, if you know of another way of doing this, I'd be interested to hear it. We discussed it in broad terms on a face to face call, but at the time nobody came up with any alternative approaches and this is kinda the thing that worked. |
@cipherboy Just a short technical comment
Yes, the remediation has the check for `var_firewall_package, but the OVAL doesn't get the extra criteria from update_with_var_platforms() |
@cipherboy @dodys Can you please explain to me why XCCDF's rule groups can't be used for this purpose? |
@evgenyz said:
Just to clarify, you mean the existing, hierarchical groups we already provision, My understanding is getting foggy as time goes on (not doing as much XCCDF work any more...) -- but here's what I recall. Perhaps @dodys or @richardmaciel-canonical can fill in the gaps. Groups, as currently created in CaC, necessarily aren't aligned with any benchmark. They're a reasonable grouping that provides some semblance of structure, based upon external, logical criteria -- services in one, programs in another, firewalls in a third, &c -- plus nesting. Additionally, they're globally static to a XCCDF/DS -- not dynamically computed based upon which Profile is selected. Suppose we introduced groups called There's a way around this exclusivity shortcoming, though: we'd need to introduce rule extensions. Here, we could take a base definition of a rule and extend it (according to XCCDF These two properties, IMO, make this implementation better. And the trade-off between not considering the rule at all (under grouping semantics proposed in this comment) vs pass/eventually not-applicable (as proposed in the codified implementation above) are hopefully alright. |
I've added a missing commit that hopefully fixes it. Also rebased and since there were changes done to |
/retest |
@dodys: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@yuumasato is this build failure after my rebase related to PR #7808, that you mentioned about SCE checks? |
@dodys Not sure which build failure you are talking about. |
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
/ok-to-test |
@dodys: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@dodys: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Is this PR alive? Conflicts can be resolved easily, but the Python code changes to the build system will need refactoring to increase their maintainability, and it is also possible that the new CPE system that somehow unifies the system of XCCDF applicability with applicability of individual remediations may change the game a little bit. |
I'm planning on dropping the CPE changes and just push the UFW changes instead, I will create a new PR for that when the time comes. |
Description: